Fix run-pass telemetry decorator placement - #2649
Fix run-pass telemetry decorator placement#2649Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟢 Approval recommended
The change aligns run-pass with existing CLI telemetry patterns and includes a focused regression test validating the intended decorator placement.
Pull request overview
This PR fixes Olive CLI telemetry for the run-pass command by moving the @action decorator from the RunPassCommand class (which unintentionally wraps the class into a function) onto the run() method, aligning it with the established pattern used by other CLI commands.
Changes:
- Removed
@actionfromRunPassCommand’s class definition so the argparse binding remains a class (not a wrapper function). - Added
@actiontoRunPassCommand.run()so telemetry records actual command execution. - Added a regression test to ensure
RunPassCommandremains a class and thatrun()is wrapped (viafunctools.wraps/__wrapped__).
File summaries
| File | Description |
|---|---|
olive/cli/run_pass.py |
Moves @action from the command class to the run() method to correctly capture execution telemetry and preserve class binding. |
test/cli/test_run_pass_action.py |
Adds regression coverage asserting the command is still a class and the run() method is telemetry-wrapped. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Integrate the PR head while preserving recipe telemetry and multi-build execution. Resolve overlaps in olive/cli/run.py and olive/workflows/run/run.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
|
Addressed in e330451: I folded the coverage into the existing test_run_pass.py, removed the wrapper-introspection test, and added direct unit coverage for RunPassCommand.run() behavior. The commit is signed and verified. |
Describe your changes
Addresses the
run-passdecorator issue identified in #2516.RunPassCommandcurrently applies@actionto the class itself, while the other CLI commands apply it to theirrun()methods. Theactiondecorator wraps callables and therefore replaces the class binding with a wrapper function, recording construction rather than the actual command execution.This change keeps
RunPassCommandas a class and applies@actiontorun()consistently with the rest of the CLI.Tests
Adds regression coverage verifying that
RunPassCommandremains a class and that itsrun()method is wrapped by the telemetry decorator.